Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

NodeJS bindings - worker threads error demo #47

Closed
wants to merge 1 commit into from
Closed

NodeJS bindings - worker threads error demo #47

wants to merge 1 commit into from

Conversation

dapplion
Copy link
Contributor

@dapplion dapplion commented Dec 10, 2020 •
edited
Loading

Current NodeJS bindings throw an error when run on more than one worker, or in the main thread + 1 worker.

To achieve a comparable level of performance closer to the other bindings it's very important to be able to multi-thread BLS signature verification

Run node bindings/node.js/worker-threads.js

events.js:291
      throw er; // Unhandled 'error' event
      ^
Error: Cannot find module '/home/runner/work/blst/blst/runnable.js'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:815:15)
    at Function.Module._load (internal/modules/cjs/loader.js:667:27)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:60:12)
    at MessagePort.<anonymous> (internal/main/worker_thread.js:164:24)
    at MessagePort.emit (events.js:314:20)
    at MessagePort.onmessage (internal/worker/io.js:80:8)
    at MessagePort.exports.emitMessage (internal/per_context/messageport.js:11:10)
Emitted 'error' event on Worker instance at:
    at Worker.[kOnErrorMessage] (internal/worker.js:242:10)
    at Worker.[kOnMessage] (internal/worker.js:252:37)
    at MessagePort.<anonymous> (internal/worker.js:168:57)
    at MessagePort.emit (events.js:314:20)
    at MessagePort.onmessage (internal/worker/io.js:80:8)
    at MessagePort.exports.emitMessage (internal/per_context/messageport.js:11:10)
    at Worker.[kOnExit] (internal/worker.js:224:5)
    at Worker.<computed>.onexit (internal/worker.js:165:20) {
  code: 'MODULE_NOT_FOUND',
  requireStack: []
}

https://github.com/supranational/blst/pull/47/checks?check_run_id=1530217010#step:10:12

This PR is meant to demo said error with the simplest setup to hopefully assist in finding a solution.

This issue hints at a possible cause and solution but it escapes may knowledge nodejs/node#21783 (comment)

@dot-asm
Copy link
Collaborator

dot-asm commented Dec 14, 2020

I've committed couple of modification that make node.js workers work. While working on it I've noted that the script was crashing with SEGV intermittently. It doesn't anymore after handling returning this in special manner, but it's not 100% reassuring. I mean it's still should be viewed as untested ground...

However! I would argue that using node.js workers to perform same kind of parallelization as in Go and Rust is not necessarily optimal path. But what would be one? Well, there is a plan to add fully-fledged C++ binding, and thing is that whatever happens in C++ can happen in SWIG bindings. SWIG territory of passing arrays of objects is uncharted, so it's not like there is clear picture, but it ought to be more graceful solution. But don't let it discourage you from venturing into node.js workers:-)

@dapplion
Copy link
Contributor Author

Thank you so much! We'll definitely explore if it makes sense to use workers performance-wise. Maybe we can just split signature sets amongst workers and aggregate the resulting booleans instead of aggregating the context to simplify message serialization

@dapplion dapplion closed this Dec 14, 2020
@dapplion dapplion deleted the worker-threads branch December 22, 2020 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dapplion @dot-asm